Skip to content
This repository was archived by the owner on Oct 26, 2018. It is now read-only.

Experimental sync logic #40

Merged
merged 3 commits into from
Nov 24, 2015
Merged

Experimental sync logic #40

merged 3 commits into from
Nov 24, 2015

Conversation

jlongster
Copy link
Member

@mariocasciaro @kadmil @box-turtle @amccloud Looping you in because you were involved in #12 and #30.

This is an experiment to simplify things even further. So far we've had trouble being compatible with HashHistory and also triggering multiple updates of the same URL when transitioning. This should fix all of that.

This is also fixes an issue where multiple calls to updatePath should trigger multiple route updates. Right now if you click the same <Link> multiple times it does trigger a route update each time, even though it's going to the same URL. Previous behavior would not do this, but with this PR it does.

Can everyone try out this PR and let me know of any obvious problems? I've tested it in simple cases so far.

@mariocasciaro
Copy link
Contributor

I tried your experimental branch in my project, so far so good, it seems to be working fine.

@ZucchiniZe
Copy link

Just tried this branch and it looks like it is working

@ZucchiniZe
Copy link

Also, you probably want to change the main property in the package.json from lib/index to src/index

@amccloud
Copy link

@ZucchiniZe lib/index is correct. Since you installed from github you need to run npm run build

@ZucchiniZe
Copy link

Ah, that makes sense, thank you!

@amccloud
Copy link

@jlongster works for me 👍

@jlongster
Copy link
Member Author

Awesome! I think I'm going to land a simple test suite along with this so we can start adding tests over time as well. Still want to make sure this doesn't break anything.

@heydemo
Copy link

heydemo commented Nov 22, 2015

@amccloud thanks for the heads up - Using this branch and works perfectly

@n1k0
Copy link

n1k0 commented Nov 23, 2015

Just confirming this patch makes the lib usable for me :)

@jlongster
Copy link
Member Author

I just added some tests. Let's go ahead and land this and see how it goes.

jlongster added a commit that referenced this pull request Nov 24, 2015
@jlongster jlongster merged commit 46c3228 into master Nov 24, 2015
@MrEfrem
Copy link

MrEfrem commented Nov 24, 2015

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants